-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix: Make metadata mutable in AzureVectorStore (Azure AI Search) #4131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…tance Avoid UnsupportedOperationException by ensuring metadata is always a LinkedHashMap. Add `parseMetadataToMutable` helper and use it in similarity search path. Relates to spring-projects#4117 Signed-off-by: Jinwoo Lee <[email protected]>
Add `AzureVectorStoreMetadataParsingTests` to verify: - valid JSON → mutable LinkedHashMap copy - blank/invalid JSON → empty LinkedHashMap - can inject `distance` without throwing UnsupportedOperationException These tests fail on the previous implementation and pass with the fix, guarding against regressions. Relates to spring-projectsGH-4117 Signed-off-by: Jinwoo Lee <[email protected]>
|
|
||
| static Map<String, Object> parseMetadataToMutable(@Nullable String metadataJson) { | ||
| if (!StringUtils.hasText(metadataJson)) { | ||
| return new LinkedHashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a particular reason you used a LinkedHashMap?
It seems like a regular HashMap would work just as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the question! I used LinkedHashMap to preserve iteration order so that the parsed JSON key order is kept and the injected distance appears last. This gives deterministic logging/serialization and makes debugging easier. There’s no functional dependency on ordering though—if the project prefers HashMap, I’m happy to switch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it doesn’t affect the outcome, there may not be a significant difference, but since HashMap is more efficient, it seems better to change it to a HashMap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great point—updated to use HashMap since ordering isn’t required. The helper now returns a new HashMap<> and tests still pass. PTAL. ✅ (4b44359)
|
At the top of the class, please add your name along with the |
…oMutable We don’t rely on insertion order for metadata, so a regular HashMap is sufficient. This change addresses reviewer feedback while still ensuring the returned map is always mutable by creating a defensive copy or returning an empty instance. Relates to spring-projects#4117 Signed-off-by: Jinwoo Lee <[email protected]>
...re/src/test/java/org/springframework/ai/vectorstore/azure/AzureVectorStoreMetadataTests.java
Outdated
Show resolved
Hide resolved
|
Please add copyright to top of |
- Add contributor name with @author tag to AzureVectorStore (per reviewer feedback). - Add brief class-level Javadoc to AzureVectorStoreMetadataTests to clarify purpose. - No functional changes. Refs spring-projects#4117. Signed-off-by: Jinwoo Lee <[email protected]>
...re/src/test/java/org/springframework/ai/vectorstore/azure/AzureVectorStoreMetadataTests.java
Outdated
Show resolved
Hide resolved
Add the standard copyright/license header to `AzureVectorStoreMetadataTests` to satisfy Spring’s checkstyle and spring-javaformat requirements. No functional changes. Signed-off-by: Jinwoo Lee <[email protected]>
* fix(azure): parse metadata JSON into mutable Map before injecting distance Avoid UnsupportedOperationException by ensuring metadata is always a LinkedHashMap. Add `parseMetadataToMutable` helper and use it in similarity search path. * test(azure): add unit tests for mutable metadata parsing and distance Add `AzureVectorStoreMetadataParsingTests` to verify: - valid JSON → mutable LinkedHashMap copy - blank/invalid JSON → empty LinkedHashMap - can inject `distance` without throwing UnsupportedOperationException These tests fail on the previous implementation and pass with the fix, guarding against regressions. * refactor(azure): replace LinkedHashMap with HashMap in parseMetadataToMutable We don’t rely on insertion order for metadata, so a regular HashMap is sufficient. This change addresses reviewer feedback while still ensuring the returned map is always mutable by creating a defensive copy or returning an empty instance. Fixes #4117 Signed-off-by: Jinwoo Lee <[email protected]> (cherry picked from commit f3962a6)
…ing-projects#4131) * fix(azure): parse metadata JSON into mutable Map before injecting distance Avoid UnsupportedOperationException by ensuring metadata is always a LinkedHashMap. Add `parseMetadataToMutable` helper and use it in similarity search path. * test(azure): add unit tests for mutable metadata parsing and distance Add `AzureVectorStoreMetadataParsingTests` to verify: - valid JSON → mutable LinkedHashMap copy - blank/invalid JSON → empty LinkedHashMap - can inject `distance` without throwing UnsupportedOperationException These tests fail on the previous implementation and pass with the fix, guarding against regressions. * refactor(azure): replace LinkedHashMap with HashMap in parseMetadataToMutable We don’t rely on insertion order for metadata, so a regular HashMap is sufficient. This change addresses reviewer feedback while still ensuring the returned map is always mutable by creating a defensive copy or returning an empty instance. Auto-cherry-pick to 1.0.x Fixes spring-projects#4117 Signed-off-by: Jinwoo Lee <[email protected]>
…ing-projects#4131) * fix(azure): parse metadata JSON into mutable Map before injecting distance Avoid UnsupportedOperationException by ensuring metadata is always a LinkedHashMap. Add `parseMetadataToMutable` helper and use it in similarity search path. * test(azure): add unit tests for mutable metadata parsing and distance Add `AzureVectorStoreMetadataParsingTests` to verify: - valid JSON → mutable LinkedHashMap copy - blank/invalid JSON → empty LinkedHashMap - can inject `distance` without throwing UnsupportedOperationException These tests fail on the previous implementation and pass with the fix, guarding against regressions. * refactor(azure): replace LinkedHashMap with HashMap in parseMetadataToMutable We don’t rely on insertion order for metadata, so a regular HashMap is sufficient. This change addresses reviewer feedback while still ensuring the returned map is always mutable by creating a defensive copy or returning an empty instance. Auto-cherry-pick to 1.0.x Fixes spring-projects#4117 Signed-off-by: Jinwoo Lee <[email protected]>
…ing-projects#4131) * fix(azure): parse metadata JSON into mutable Map before injecting distance Avoid UnsupportedOperationException by ensuring metadata is always a LinkedHashMap. Add `parseMetadataToMutable` helper and use it in similarity search path. * test(azure): add unit tests for mutable metadata parsing and distance Add `AzureVectorStoreMetadataParsingTests` to verify: - valid JSON → mutable LinkedHashMap copy - blank/invalid JSON → empty LinkedHashMap - can inject `distance` without throwing UnsupportedOperationException These tests fail on the previous implementation and pass with the fix, guarding against regressions. * refactor(azure): replace LinkedHashMap with HashMap in parseMetadataToMutable We don’t rely on insertion order for metadata, so a regular HashMap is sufficient. This change addresses reviewer feedback while still ensuring the returned map is always mutable by creating a defensive copy or returning an empty instance. Auto-cherry-pick to 1.0.x Fixes spring-projects#4117 Signed-off-by: Jinwoo Lee <[email protected]> Signed-off-by: 家娃 <[email protected]>
Summary
When running a similarity search against Azure AI Search,
AzureVectorStoreinjects the computed distance into the document metadata. For results with no metadata, the previous code usedMap.of()(immutable), sometadata.put(...)could throw anUnsupportedOperationException.This PR ensures the metadata map is always mutable before enrichment.
Root Cause
What’s changed
parseMetadataToMutable(@Nullable String)helper inAzureVectorStoreMap<String,Object>and always returns a newLinkedHashMap<>LinkedHashMapwhen input is blank or parsing returnsnulldoSimilaritySearch(...)with the helper and then appends thedistancefieldBehavior
Tests
AzureVectorStoreMetadataTeststo verify the helper behavior:distance)Module
vector-stores/spring-ai-azure-storeRelated
Fixes #4117